float tests: deduplicate min, max, and rounding tests #142243

RalfJung · 2025-06-09T12:01:28Z

Best reviewed commit-by-commit.

Use assert_biteq! in the mod.rs tests. This requires some trickery to make shadowing macros with imports work.
The min, max, minimum, maximum tests in tests/floats/f*.rs are entirely subsumed by what we already have in tests/float/mod.rs, so I just removed them.
The rounding tests (floor etc) in f*.rs had more test points, so I copied them over. They didn't have 0.5 and -0.5 though which seem like interesting points in particular regarding the sign of the resulting zero if that's what it sounds to, and they didn't max min/max/inf/nan tests, so this was really a merger of both tests.

RalfJung · 2025-06-09T12:02:58Z

library/coretests/tests/floats/mod.rs

+        assert_biteq!((-0.0 as Float).midpoint(-0.0), -0.0);
+        assert_biteq!((-5.0 as Float).midpoint(5.0), 0.0);
+        assert_biteq!(Float::MAX.midpoint(Float::MIN), 0.0);
+        assert_biteq!(Float::MIN.midpoint(Float::MAX), 0.0);


The old test seemed to assume that the result of MIN.midpoint(MAX) should be -0.0, but actually we return 0.0.

The midpoint function in C++ with clang and GCC also seems to return +0.0, so this should be fine. We don't guarantee the sign of zero anyway for midpoint.

RalfJung · 2025-06-09T12:27:07Z

library/coretests/tests/floats/mod.rs

-        assert_eq!(Float::MAX.ceil(), Float::MAX);
-        assert_eq!(Float::MIN.ceil(), Float::MIN);
-        assert_eq!(Float::MIN_POSITIVE.ceil(), 1.0);
-        assert_eq!((-Float::MIN_POSITIVE).ceil(), 0.0);


A bunch of the -Float::MIN_POSITIVE rounding tests were missing the - as well.

library/coretests/tests/floats/f32.rs

tgross35

LGTM, thanks for the cleanup. One question then r=me.

library/coretests/tests/floats/f32.rs

tgross35 · 2025-06-13T05:25:12Z

library/coretests/tests/floats/mod.rs

@@ -14,10 +37,11 @@ macro_rules! assert_approx_eq {
        );
    }};
 }
+pub(crate) use assert_approx_eq_ as assert_approx_eq;


What is the reason for the trailing _ rather than just pub(crate) use assert_approx_eq;? I assume there may be some name conflicts.

I got an error about name clashes:

error[E0659]: `assert_approx_eq` is ambiguous --> library/coretests/tests/floats/mod.rs:681:9 | 681 | assert_approx_eq!((-1.7 as Float).fract(), -0.7); | ^^^^^^^^^^^^^^^^ ambiguous name | = note: ambiguous because of a conflict between a `macro_rules` name and a non-`macro_rules` name from another module

I think I'll pick actually proper names for both runtime and const macros and use imports for name management, that's probably clearer.

I restructured the macros a bit to make it more clear (I think), please take a look. Putting the const ones in a module doesn't work out that well since (a) macros have their own special global namespace, and (b) we can't glob-import them anyway.

Yeah that looks good to me. Maybe just add a breadcrumb comment at the use {assert_*_const as assert_*} remapping? Also if you don't mind grabbing it while you're here, I notice the preexisting doc example for the float_test macro is missing its closing /// ``` .

I still can't wrap my head around the way macro namespaces work sometimes...

There's already a comment there. What would you like to have added to it?

I still can't wrap my head around the way macro namespaces work sometimes...

Yeah.^^ I wonder if we can get rid of it in a future edition...

There is a comment at the assert_approx_eq_rt as assert_approx_eq version mentioning that the rt version is used by default so const can shadow it, I was just thinking to mirror this at the place we shadow it (lines 193-194). But it's noncritical, r=me with or without it.

tgross35

Oops forgot the ✅

RalfJung · 2025-06-13T14:05:09Z

@bors r=tgross35

bors · 2025-06-13T14:05:12Z

📌 Commit 25ec235 has been approved by tgross35

It is now in the queue for this repository.

float tests: deduplicate min, max, and rounding tests Part of rust-lang#141726 Best reviewed commit-by-commit. - Use `assert_biteq!` in the `mod.rs` tests. This requires some trickery to make shadowing macros with imports work. - The min, max, minimum, maximum tests in `tests/floats/f*.rs` are entirely subsumed by what we already have in `tests/float/mod.rs`, so I just removed them. - The rounding tests (floor etc) in `f*.rs` had more test points, so I copied them over. They didn't have `0.5` and `-0.5` though which seem like interesting points in particular regarding the sign of the resulting zero if that's what it sounds to, and they didn't max min/max/inf/nan tests, so this was really a merger of both tests. r? `@tgross35`

Rollup of 9 pull requests Successful merges: - #140593 (Temporary lifetime extension through tuple struct and tuple variant constructors) - #141399 ([rustdoc] Give more information into extracted doctest information) - #141493 (Delegate `<SocketAddr as Debug>` to `ByteStr`) - #141811 (Unimplement unsized_locals) - #142243 (float tests: deduplicate min, max, and rounding tests) - #142464 (variadic functions: remove list of supported ABIs from error) - #142477 (Fix incorrect suggestion when calling an associated type with a type anchor) - #142484 (Remove unneeded lifetime bound from signature of BTreeSet::extract_if) - #142489 (Update the `compiler-builtins` subtree) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #142243 - RalfJung:float-test-dedup, r=tgross35 float tests: deduplicate min, max, and rounding tests Part of #141726 Best reviewed commit-by-commit. - Use `assert_biteq!` in the `mod.rs` tests. This requires some trickery to make shadowing macros with imports work. - The min, max, minimum, maximum tests in `tests/floats/f*.rs` are entirely subsumed by what we already have in `tests/float/mod.rs`, so I just removed them. - The rounding tests (floor etc) in `f*.rs` had more test points, so I copied them over. They didn't have `0.5` and `-0.5` though which seem like interesting points in particular regarding the sign of the resulting zero if that's what it sounds to, and they didn't max min/max/inf/nan tests, so this was really a merger of both tests. r? ``@tgross35``

Rollup of 9 pull requests Successful merges: - rust-lang/rust#140593 (Temporary lifetime extension through tuple struct and tuple variant constructors) - rust-lang/rust#141399 ([rustdoc] Give more information into extracted doctest information) - rust-lang/rust#141493 (Delegate `<SocketAddr as Debug>` to `ByteStr`) - rust-lang/rust#141811 (Unimplement unsized_locals) - rust-lang/rust#142243 (float tests: deduplicate min, max, and rounding tests) - rust-lang/rust#142464 (variadic functions: remove list of supported ABIs from error) - rust-lang/rust#142477 (Fix incorrect suggestion when calling an associated type with a type anchor) - rust-lang/rust#142484 (Remove unneeded lifetime bound from signature of BTreeSet::extract_if) - rust-lang/rust#142489 (Update the `compiler-builtins` subtree) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned tgross35 Jun 9, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 9, 2025

RalfJung commented Jun 9, 2025

View reviewed changes

float tests: use assert_biteq in more places

20cf8ca

RalfJung force-pushed the float-test-dedup branch from dc526c1 to 7171984 Compare June 9, 2025 12:18

RalfJung added 3 commits June 9, 2025 14:24

float tests: deduplicate min, max, and rounding tests

79cb013

make the default float comparison tolerance type-dependent

2766b77

float midpoint tests: add missing NAN cases

1b6bb45

RalfJung force-pushed the float-test-dedup branch from 7171984 to 1b6bb45 Compare June 9, 2025 12:24

RalfJung commented Jun 9, 2025

View reviewed changes

library/coretests/tests/floats/f32.rs Show resolved Hide resolved

tgross35 reviewed Jun 13, 2025

View reviewed changes

tgross35 approved these changes Jun 13, 2025

View reviewed changes

RalfJung force-pushed the float-test-dedup branch 2 times, most recently from a2584a4 to 6da4f58 Compare June 13, 2025 07:06

tweak runtime/const macro management

25ec235

RalfJung force-pushed the float-test-dedup branch from 6da4f58 to 25ec235 Compare June 13, 2025 08:23

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2025

jhpratt mentioned this pull request Jun 14, 2025

Rollup of 10 pull requests #142490

Closed

matthiaskrgr mentioned this pull request Jun 14, 2025

Rollup of 9 pull requests #142492

Merged

bors merged commit 4cf4473 into rust-lang:master Jun 14, 2025
10 checks passed

rustbot added this to the 1.89.0 milestone Jun 14, 2025

RalfJung deleted the float-test-dedup branch June 14, 2025 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

float tests: deduplicate min, max, and rounding tests #142243

float tests: deduplicate min, max, and rounding tests #142243

Uh oh!

RalfJung commented Jun 9, 2025 •

edited

Loading

Uh oh!

RalfJung Jun 9, 2025 •

edited

Loading

Uh oh!

Urgau Jun 9, 2025

Uh oh!

RalfJung Jun 9, 2025

Uh oh!

Uh oh!

tgross35 left a comment

Uh oh!

Uh oh!

tgross35 Jun 13, 2025

Uh oh!

RalfJung Jun 13, 2025

Uh oh!

RalfJung Jun 13, 2025

Uh oh!

tgross35 Jun 13, 2025

Uh oh!

RalfJung Jun 13, 2025

Uh oh!

tgross35 Jun 13, 2025

Uh oh!

tgross35 left a comment

Uh oh!

RalfJung commented Jun 13, 2025

Uh oh!

bors commented Jun 13, 2025

Uh oh!

Uh oh!

Uh oh!

float tests: deduplicate min, max, and rounding tests #142243

float tests: deduplicate min, max, and rounding tests #142243

Uh oh!

Conversation

RalfJung commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 left a comment

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jun 13, 2025

Uh oh!

bors commented Jun 13, 2025

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Jun 9, 2025 •

edited

Loading

RalfJung Jun 9, 2025 •

edited

Loading